-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove whatwg-fetch #1134
Remove whatwg-fetch #1134
Conversation
@@ -1,3 +1,5 @@ | |||
import './fetch'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be imported in the network interface only - that's the only part that depends on fetch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whatwg-fetch
import was in both transport/networkInterface.js
and transport/batchedNetworkInterface.js
. Both of which are also imported into this file, so the same behavior would occur if we moved this to one or the other files.
With that in mind, would you still like it moved to the network interface, and if so which one? Or both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's a good point. We don't have any facility for not loading part of the code.
Given that, can we print the warning only when createNetworkInterface
is called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, moved it.
@calebmer Looks great!! Can you create a PR on the docs to make sure we don't forget to tell people that they have to install a fetch implementation now? |
@helfer 👍
|
As per an earlier discussion, this PR removes
whatwg-fetch
and replaces it with a runtime warning if a globalfetch
implementation was not found 👍Looks like this one kills another 15% in file size taking us down to 21.7 kB gzipped.